Hi Sagun,

I am sceptical if this change is correct.

If ilContext::getType() equals ilContext::CONTEXT_SOAP, then this means the requested PHP script is public/soap/server.php.

If the ilContext::getType() in the script execution does not equal ilContext::CONTEXT_SOAP, \ilSoapAdministration::initAuth is executed in another context, namely the identical PHP request the cascading container copy process was initiated in, see the fallback in: \ilContainer::cloneAllObject. The executed PHP script in this context is public/ilias.php.

...
        $new_session_id = ilSession::_duplicate($session_id);
...
        if ($soap_client->init()) {
            ilLoggerFactory::getLogger('obj')->info('Calling soap clone method');
            $res = $soap_client->call('ilClone', [$new_session_id . '::' . $client_id, $copy_id]);
        } else {
            ilLoggerFactory::getLogger('obj')->warning('SOAP clone call failed. Calling clone method manually');
            $wizard_options->disableSOAP();
            $wizard_options->read();
            $res = ilSoapFunctions::ilClone($new_session_id . '::' . $client_id, $copy_id); // <------------------------------
        }
...

If we now stop re-initializing ILIAS and re-setting the PHP session for this fallback context (the else-branch, which then leads to ilContext::getType() !== ilContext::CONTEXT_SOAP), this will log out the current user. The SOAP client (in this case, ILIAS itself) calls ilSoapFunctions::ilClone(...) with a new PHP session id, so \ilSoapAdministration::initAuth has to ensure this new PHP session id is set and the user context is re-initialized) after the old PHP session has been destroyed.

If the ILIAS SOAP server has been called via the public/soap/server.php script (ilContext::getType() === ilContext::CONTEXT_SOAP) there should be no need to initialize the ILIAS user context again and also no need to set an HTTP cookie, since this will be done later in \ilSoapAdministration::initIlias (or is yet done – and this MIGHT be a/the introduced problem in ILIAS 11 – with the call of the entry_point function in https://github.com/ILIAS-eLearning/ILIAS/blob/release_11/components/ILIAS/soap/resources/soap/server.php#L30).

So if this PR here gets merged, https://mantis.ilias.de/view.php?id=43882 will occur again.

Best regards,
Michael

Hi @mjansenDatabay ,

Thank you for the explanation in detail.
If i understood correcty, in SOAP context, initAuth() correctly skips reInitUser().
The guard ilContext::getType() !== ilContext::CONTEXT_SOAP is important to prevent current user getting logged out.

What i understood is we need to re-initialize the User context. Calling reInitUser() after initIlias() seem to fix the issue: either directly in ilClone() or inside initIlias() itself.

In components/ILIAS/soap/classes/class.ilSoapUtils.php:

public function ilClone(string $sid, int $copy_identifier)
{
$this->initAuth($sid);
$this->initIlias();
$this->reInitUser(); <------ this will re-initialize the user and the copy process goes through.

But does this mean, for every soap functions we might have to call reInitUser after calling initILIAS (although i can see 4-5 places in class.ilSoapUtils.php). Or maybe call once inside initILIAS in SoapAdministration so change in only one place. Please correct me if my analysis here is off the track.
Thank you :)

Kind regards,
Sagun

Hi Sagun,

If i understood correcty, in SOAP context, initAuth() correctly skips reInitUser().

Yes, because recursive calls in the copy/clone process should lead to new SOAP HTTP requests (resulting in a new PHP process with new initialization etc.).

The guard ilContext::getType() !== ilContext::CONTEXT_SOAP is important to prevent current user getting logged out.

Yes, if the SOAP client cannot be initialized (for instance, due to network issues in the server environment), ILIAS calls ilSoapFunctions::ilClone recursively, which leads to multiple calls of ilSoapFunctions::ilClone (see: \ilContainer::cloneAllObject and ilSoapFunctions::callNextNode) in the same PHP process. Since \ilContainer::cloneAllObject passed a new PHPSESSID and \ilSoapAdministration::initAuth destroys the current session if it differs from the new/passed one, we'll have to re-initialize the global user instance and force ILIAS to send a new HTTP PHPSESSID cookie to thr browser. Otherwise, the user will be logged out.

Option 1) Maybe you could try this in \ilSoapAdministration::initAuth:

...
        ilInitialisation::reInitUser();
        if (ilContext::getType() !== ilContext::CONTEXT_SOAP) {
            ilUtil::setCookie(session_name(), $sid);
        }
...

This central approach re-initializes the user in every execution context but only sends the HTTP cookie in case ilContext::getType() !== ilContext::CONTEXT_SOAP (means: the current script is public/ilias.php, not public/soap/server.php).

Option 2) I guess(!) the issue with ILIAS 11 occurs because we now have to invoke the entry_point(...) function in all of our ILIAS endpoint scripts, which leads to an immediate/early initialization. I suggest to set ilContext::init(ilContext::CONTEXT_SOAP); before invoking entry_point(...) in components/ILIAS/soap/resources/soap/server.php.

...
ilContext::init(ilContext::CONTEXT_SOAP);

entry_point('ILIAS Legacy Initialisation Adapter');
...

The context should always be set before(!) we initialize ILIAS.

Maybe this does the trick. Maybe we need a combination of Option 2 and Option 1.

We should avoid applying changes to all SOAP service methods by introducing $this->reInitUser(); to every method.

Best regards,
Michael

Hi @mjansenDatabay ,

Thank you for the feedback :)

Yes, in my opinion we need combination of both although option 1 itself seem to already fix the issue but like you mentioned, the context should always be set before we initialize ILIAS. I could see the same design in other places too. For example: ILIAS/components/ILIAS/WebDAV/resources/webdav.php.

Thank you once again. :)

Kind regards,
Sagun

Hi @GitHamo ,

Do you agree with the changes ? Solution is in place. I have tested as well. Can you please provide your feedback ?
Thank you :)

kind regards,
Sagun

Hi @GitHamo ,

Do you agree with the changes ? Solution is in place. I have tested as well. Can you please provide your feedback ? Thank you :)

kind regards, Sagun

Hi Sagun,

I agree with the changes, but please check if the $this->reInitUser(); call in \ilAbstractSoapMethod::initIliasAndCheckSession is still necessary after integratring this PR. I don't think so.

Best regards,
Michael

Hi again @mjansenDatabay ,

Thank you for suggestion.
I tested quickly by removing the reInitUser call in \ilAbstractSoapMethod::initIliasAndCheckSession and everything is running alright. This infact seems redundant and can be removed since we are initializing the user in initAuth regardless. And i think better to make the changes in this PR only so later we don't forget about it.

If @GitHamo also agrees with previous changes and this as well, i will then move ahead with merging and creating PR for trunk as well.

Thank u once again :)

kind regards,
Sagun

And i think better to make the changes in this PR only so later we don't forget about it.

I agree!

Argh